test: Auth 서비스 단위 테스트 추가 + 보안 버그 2건 수정 + Apple 서명검증 dev 반영 - #221
Merged
Conversation
AppleSignInService.getSocialInfo()가 클라이언트가 보낸 Apple ID 토큰을 SignedJWT.parse()로 파싱만 하고 암호학적 서명 검증을 하지 않고 있었음. 발급처(iss)/대상(aud)/만료시간/이메일 인증 여부 같은 클레임 값만 확인했는데, 이 값들은 서명 검증 없이는 클라이언트가 얼마든지 임의로 채울 수 있어 실제로는 애플 로그인 없이도 provider=APPLE로 정식 로그인/토큰 발급이 가능한 상태였음. Apple의 공개키(JWKS, https://appleid.apple.com/auth/keys)로 RS256 서명을 검증하도록 수정. nimbus-jose-jwt(기존 의존성)의 JWSVerificationKeySelector + DefaultJWTProcessor를 사용해 서명이 유효한 토큰만 클레임을 신뢰하도록 함.
AuthService(getNewToken/signIn) 13개, AppleSignInService(P8 키 파싱/ 잘못된 idToken) 3개, 총 16개 신규 테스트. 테스트 작성 중 발견해서 수정한 버그: 1. [Critical] AuthService.getNewToken: refreshToken을 재발급받을 때 Redis에 "무언가 저장돼 있는지"만 확인하고, 요청으로 들어온 refreshToken이 실제로 그 저장된 값과 일치하는지는 비교하지 않고 있었음. 재로그인 등으로 이미 새 refreshToken이 발급되어 Redis 값이 교체된 이후에도, 예전 refreshToken이 만료 전이기만 하면 계속 accessToken 재발급에 쓰일 수 있었던 상태 — refreshToken 무효화가 사실상 작동하지 않고 있었음. 저장된 값과 요청값을 직접 비교하도록 수정. 2. KakaoSignInService.getSocialInfo: 카카오 API 호출(RestTemplate.exchange)이 try-catch 밖에 있어서, 카카오 토큰이 만료/무효해 카카오 서버가 4xx를 반환하면(흔한 케이스) RestTemplate이 던지는 예외가 그대로 새어나가 401(UnauthorizedException) 대신 500으로 처리되고 있었음. API 호출을 try 블록 안으로 이동. 추가로 signIn()의 디버그용 System.out.println 제거. ## dev에 누락돼있던 기존 보안 수정 반영 AppleSignInService의 Apple ID 토큰 서명 검증 로직이 dev에는 없었음. PR #202(main으로 직접 hotfix, 2026-07-28)로 이미 고쳐졌던 건인데 dev로는 한 번도 반영이 안 된 채 남아있었음 — dev에서 계속 개발하면 서명 검증 없이 파싱만 하는 취약한 버전으로 되돌아간 상태였음. main의 c74b4ab 커밋을 그대로 cherry-pick해서 dev에도 반영.
📝 WalkthroughWalkthroughAuthentication services now verify Apple JWT signatures, normalize Kakao request failures, and require refresh-token equality with the Redis value. Tests cover Apple token handling, refresh-token validation, provider routing, user creation, and existing-user sign-in. ChangesAuthentication validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AppleSignInService
participant AppleJWKS
participant NimbusJWTVerifier
AppleSignInService->>AppleJWKS: Fetch Apple signing keys
AppleSignInService->>NimbusJWTVerifier: Verify RS256 JWT signature
NimbusJWTVerifier-->>AppleSignInService: Return verified claims
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/org/runnect/server/auth/service/AppleSignInService.java (1)
111-118: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse an application-scoped JWKS source.
verifySignatureAndGetClaims()creates a newRemoteJWKSetper sign-in, which loses the library’s internal JWK cache and repeats JWKS fetches for each request. Use one injectedJWKSource/JWT processor with explicit retrieval settings and reuse it for Apple ID token processing.Make the JWKS source injectable in tests.
AppleSignInServicecurrently hard-codes JWKS verification and network calls. Inject a controlledJWKSourceintogetSocialInfo(), or inject the JWT processor/key selector, so tests cover valid RS256 tokens and invalid signatures without Apple’s server.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/runnect/server/auth/service/AppleSignInService.java` around lines 111 - 118, Refactor AppleSignInService.verifySignatureAndGetClaims() to reuse an application-scoped, injectable JWKSource or JWT processor/key selector configured with explicit retrieval settings instead of constructing RemoteJWKSet per request; update AppleSignInServiceTest.java to inject a controlled source and cover valid RS256 tokens and invalid signatures without network calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/runnect/server/auth/service/AppleSignInService.java`:
- Around line 100-106: Update exception handling in AppleSignInService and
KakaoSignInService so connectivity, timeout, JWKS retrieval, and provider 5xx
failures map to the existing retryable provider error, while signature, parsing,
and invalid-credential failures remain INVALID_*_ID_TOKEN_EXCEPTION; preserve
UnauthorizedException propagation and classify each failure using the
provider/client exception types already used by these services.
In `@src/main/java/org/runnect/server/auth/service/AuthService.java`:
- Around line 55-60: Update the refresh-token reissue flow in AuthService around
storedRefreshToken and issuedAccessToken to atomically compare-and-rotate the
Redis session state, so a token cannot reissue after a concurrent sign-in
replaces it. Ensure the rotation preserves the current refresh-token value only
when it still matches, and invalidate associated access tokens when session
rotation occurs; reject the request when the atomic check fails.
---
Nitpick comments:
In `@src/main/java/org/runnect/server/auth/service/AppleSignInService.java`:
- Around line 111-118: Refactor AppleSignInService.verifySignatureAndGetClaims()
to reuse an application-scoped, injectable JWKSource or JWT processor/key
selector configured with explicit retrieval settings instead of constructing
RemoteJWKSet per request; update AppleSignInServiceTest.java to inject a
controlled source and cover valid RS256 tokens and invalid signatures without
network calls.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 97408f39-6b5f-4a95-bfab-d63b34b852c7
📒 Files selected for processing (5)
src/main/java/org/runnect/server/auth/service/AppleSignInService.javasrc/main/java/org/runnect/server/auth/service/AuthService.javasrc/main/java/org/runnect/server/auth/service/KakaoSignInService.javasrc/test/java/org/runnect/server/auth/service/AppleSignInServiceTest.javasrc/test/java/org/runnect/server/auth/service/AuthServiceTest.java
Comment on lines
+100
to
106
| }catch (UnauthorizedException e){ | ||
| throw e; | ||
| }catch (Exception e){ | ||
| // 서명 검증 실패(BadJOSEException), 파싱 실패(ParseException), JWKS 조회 실패(JOSEException) 등 | ||
| // 위조/변조된 토큰 또는 애플 검증 자체가 불가능한 경우 전부 동일하게 처리 | ||
| throw new UnauthorizedException(ErrorStatus.INVALID_APPLE_ID_TOKEN_EXCEPTION, | ||
| ErrorStatus.INVALID_APPLE_ID_TOKEN_EXCEPTION.getMessage()); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/main/java/org/runnect/server --items all --type class,method
rg -n -C 4 \
'UnauthorizedException|RemoteKeySourceException|ResourceAccessException|HttpServerErrorException|`@ExceptionHandler`' \
src/main/javaRepository: Runnect/Runnect-Spring-Boot-Server
Length of output: 37295
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== AppleSignInService.java =="
cat -n src/main/java/org/runnect/server/auth/service/AppleSignInService.java | sed -n '1,180p'
echo "== KakaoSignInService.java =="
cat -n src/main/java/org/runnect/server/auth/service/KakaoSignInService.java | sed -n '1,90p'
echo "== Global exception handlers =="
cat -n src/main/java/org/runnect/server/common/advice/ControllerExceptionAdvice.java | sed -n '1,130p'
echo "== ErrorStatus references =="
rg -n -C 2 'INVALID_APPLE_ID_TOKEN_EXCEPTION|INVALID_KAKAO_ID_TOKEN_EXCEPTION|INVALID_GOOGLE_ID_TOKEN_EXCEPTION|KAKAO|APPLE|GOOGLE' src/main/java src/test/java | sed -n '1,220p'Repository: Runnect/Runnect-Spring-Boot-Server
Length of output: 40437
Do not treat provider outages as invalid-provider credentials.
These catch blocks throw INVALID_*_ID_TOKEN_EXCEPTION for any Exception, including JWKS retrieval/network failures and Kakao API 5xx/network failures. Return a retryable provider error for connectivity/timeout/server failures while keeping signature or invalid-credential failures as invalid-token errors.
📍 Affects 2 files
src/main/java/org/runnect/server/auth/service/AppleSignInService.java#L100-L106(this comment)src/main/java/org/runnect/server/auth/service/KakaoSignInService.java#L37-L49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/org/runnect/server/auth/service/AppleSignInService.java` around
lines 100 - 106, Update exception handling in AppleSignInService and
KakaoSignInService so connectivity, timeout, JWKS retrieval, and provider 5xx
failures map to the existing retryable provider error, while signature, parsing,
and invalid-credential failures remain INVALID_*_ID_TOKEN_EXCEPTION; preserve
UnauthorizedException propagation and classify each failure using the
provider/client exception types already used by these services.
Comment on lines
+55
to
+60
| final String storedRefreshToken = redisService.getValuesByKey(String.valueOf(userId)); | ||
| // Redis에 저장된 최신 refreshToken과 실제로 일치하는지까지 확인한다. | ||
| // (단순히 "뭔가 저장돼 있는지"만 보면, 재로그인 등으로 이미 무효화된 | ||
| // 예전 refreshToken도 계속 accessToken 재발급에 쓰일 수 있었음) | ||
| if (storedRefreshToken == null || storedRefreshToken.isBlank() || !storedRefreshToken.equals(refreshToken)) { | ||
| //탈취되었거나 이미 무효화된 refreshToken인 경우 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/main/java/org/runnect/server/auth/service/AuthService.java --items all
ast-grep outline src/main/java/org/runnect/server/config/jwt/JwtService.java --items all
ast-grep outline src/main/java/org/runnect/server/config/redis/RedisService.java --items all
rg -n -C 4 'getNewToken|issuedAccessToken|issuedRefreshToken|setValues|getValuesByKey' \
src/main/java/org/runnect/server/auth/service/AuthService.java \
src/main/java/org/runnect/server/config/jwt/JwtService.java \
src/main/java/org/runnect/server/config/redis/RedisService.javaRepository: Runnect/Runnect-Spring-Boot-Server
Length of output: 9941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,125p' src/main/java/org/runnect/server/auth/service/AuthService.java | cat -n
sed -n '1,90p' src/main/java/org/runnect/server/config/jwt/JwtService.java | cat -n
sed -n '1,55p' src/main/java/org/runnect/server/config/redis/RedisService.java | cat -n
rg -n 'setIfAbsent|setIfPresent|compareAndSet|opsForValue\(\)\.setIfAbsent|opsForValue\(\)\.compareAndSet|lock|sessionVersion|session-version|sessionVersion|invalidate|issuedRefreshToken|refreshToken' src/main/java src/test/javaRepository: Runnect/Runnect-Spring-Boot-Server
Length of output: 20604
Broken Authentication (CWE-367): Time-of-check Time-of-use (TOCTOU) Race Condition
Reachability: External · Exploitability: Moderate
Make refresh-token reissue atomic with Redis rotation.
A stolen/old refresh token can read the current Redis value, then a concurrent sign-in can replace that value before issuedAccessToken runs. The stale refresh token still receives a new access token.
Use atomic Redis state rotation for refresh-token reissue, such as comparing against the current stored value, writing an unchanged version key, and invalidating access tokens when session rotation occurs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/org/runnect/server/auth/service/AuthService.java` around lines
55 - 60, Update the refresh-token reissue flow in AuthService around
storedRefreshToken and issuedAccessToken to atomically compare-and-rotate the
Redis session state, so a token cannot reissue after a concurrent sign-in
replaces it. Ensure the rotation preserves the current refresh-token value only
when it still matches, and invalidate associated access tokens when session
rotation occurs; reject the request when the atomic check fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
작업 배경
AppleSignInService의 Apple ID 토큰 서명 검증 로직이 dev에는 없었음. PR #202(2026-07-28, main으로 직접 hotfix)로 이미 고쳐졌던 취약점인데 dev로는 한 번도 반영이 안 돼서, dev 기준으로 개발을 계속하면 서명 검증 없이 클레임만 파싱하는 취약한 버전으로 되돌아가는 상태였음. main의c74b4ab커밋을 그대로 cherry-pick해서 반영.변경 사항
AppleSignInServiceAuthService.getNewTokenAuthService.signInSystem.out.println제거KakaoSignInService.getSocialInfoAuthServiceTest,AppleSignInServiceTest영향 범위
getNewToken이 지금까지 Redis에 "뭔가 저장돼 있는지"만 확인하고 있어서, 재로그인 등으로 이미 무효화된 예전 refreshToken도 JWT 자체 만료 전까지는 계속 accessToken 재발급에 쓰일 수 있었음. 즉 refreshToken 무효화가 사실상 작동하지 않던 상태 — 이번 수정으로 실제 저장된 최신 토큰과 일치할 때만 재발급되도록 막힘.GoogleSignInService/KakaoSignInService는 HTTP/암호화 클라이언트를 메서드 내부에서 직접 생성해서(의존성 주입 안 됨) 네트워크 없이는 단위 테스트가 어려움 — 이번엔 다루지 않음. 필요하면 별도로 생성자 주입 리팩터링 후 테스트 추가 고려.검증 매트릭스
정상_재발급•
accessToken_무효•
refreshToken_만료•
refreshToken_무효•
클레임이_숫자가_아님•
존재하지_않는_유저redis에_저장된_값이_없음•
redis에_저장된_값과_다름신규_회원가입•
기존_유저_로그인•
애플_로그인•
닉네임_중복시_재생성•
잘못된_provider유효한_EC_비밀키면_정상적으로_파싱된다•
잘못된_형식의_비밀키면_UnauthorizedException•
idToken이_JWT_형식이_아니면_UnauthorizedExceptionTest Plan
./gradlew build전체(기존 ServerApplicationTests 포함) 통과 확인🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests